Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(GRAPHQL): Undo the breaking change and tag it as deprecated. #7602

Merged
merged 5 commits into from
Mar 18, 2021

Conversation

JatinDev543
Copy link
Contributor

@JatinDev543 JatinDev543 commented Mar 17, 2021

Fixes GRAPHQL-1119
We had this bug fix PR #7158 that also went into 20.11 release branch which disallow id argument in get queries on interfaces.
Because it was breaking change , we are now rolling back this change and mark it as deprecated, and change later.

Orignal bug
https://discuss.dgraph.io/t/id-directive-and-interfaces/11642

Workaround
So, we have disallowed @id argument from get query on interface, but the normal query still have it. So, users can change their
get queries with simple query as follows.

Interface with @id field

interface Foo {
 id: String! @id
}

We can change below query

query{
getFoo(id:"test"){
      id
 }
}

to

query{
queryFoo(filter: {id:{eq: "test"}}){ 
        id
   } 
}

Related Posts
https://discuss.dgraph.io/t/id-directive-and-interfaces/11642
https://discuss.dgraph.io/t/was-there-a-change-to-generated-queries-with-id-directives-in-v20-11-1/12591


This change is Reviewable

@github-actions github-actions bot added the area/graphql Issues related to GraphQL support on Dgraph. label Mar 17, 2021
@JatinDev543 JatinDev543 changed the title fix(GRAPHQL): Undo the breaking changes and tag fix(GRAPHQL): Undo the breaking changes and tag it as deprecated. Mar 17, 2021
@JatinDev543 JatinDev543 changed the title fix(GRAPHQL): Undo the breaking changes and tag it as deprecated. fix(GRAPHQL): Undo the breaking change and tag it as deprecated. Mar 17, 2021
Copy link
Contributor

@abhimanyusinghgaur abhimanyusinghgaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @jatindevdg and @pawanrawal)


graphql/schema/testdata/schemagen/output/interface-with-id-directive.graphql, line 460 at r1 (raw file):

xid

should we put this as @id, will be more easier for users to understand?

@JatinDev543
Copy link
Contributor Author

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @jatindevdg and @pawanrawal)

graphql/schema/testdata/schemagen/output/interface-with-id-directive.graphql, line 460 at r1 (raw file):

xid

should we put this as @id, will be more easier for users to understand?

changed

@JatinDev543 JatinDev543 merged commit ebab7d6 into release/v21.03 Mar 18, 2021
@JatinDev543 JatinDev543 deleted the jatin/undoBreakingchangesBy-GRAPHQL-886 branch March 18, 2021 06:51
aman-bansal pushed a commit that referenced this pull request Mar 25, 2021
Fixes GRAPHQL-1119
We had this bug fix PR #7158 that also went into 20.11 release branch which disallow id argument in get queries on interfaces.
Because it was breaking change , we are now rolling back this change and mark it as deprecated, and change later.

Orignal bug
https://discuss.dgraph.io/t/id-directive-and-interfaces/11642

Workaround
So, we have disallowed @id argument from get query on interface, but the normal query still have it. So, users can change their
get queries with simple query as follows.

Interface with @id field

interface Foo {
 id: String! @id
}

We can change below query

query{
getFoo(id:"test"){
      id
 }
}
to

query{
queryFoo(filter: {id:{eq: "test"}}){
        id
   }
}
Related Posts
https://discuss.dgraph.io/t/id-directive-and-interfaces/11642
https://discuss.dgraph.io/t/was-there-a-change-to-generated-queries-with-id-directives-in-v20-11-1/12591
JatinDev543 added a commit that referenced this pull request Apr 17, 2021
…oss all the implementing types. (#7710)

Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field.

Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field.

Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts.

If the interface argument is not present or its value is false then that field will be unique only for one implementing type.
But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602

Example Schema,

 interface LibraryItem {
    refID: String! @id                   //  This field is unique only for one implementing type
    itemID: String! @id(interface:true)    // This field will be unique over all the implementing types inheriting this interface
}

type Book implements LibraryItem {
    title: String
    author: String
}
Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294
harshil-goel pushed a commit that referenced this pull request Jun 14, 2023
…oss all the implementing types. (#7710)

Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field.

Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field.

Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts.

If the interface argument is not present or its value is false then that field will be unique only for one implementing type.
But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602

Example Schema,

 interface LibraryItem {
    refID: String! @id                   //  This field is unique only for one implementing type
    itemID: String! @id(interface:true)    // This field will be unique over all the implementing types inheriting this interface
}

type Book implements LibraryItem {
    title: String
    author: String
}
Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294
harshil-goel pushed a commit that referenced this pull request Jul 6, 2023
…oss all the implementing types. (#7710)

Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field.

Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field.

Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts.

If the interface argument is not present or its value is false then that field will be unique only for one implementing type.
But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602

Example Schema,

 interface LibraryItem {
    refID: String! @id                   //  This field is unique only for one implementing type
    itemID: String! @id(interface:true)    // This field will be unique over all the implementing types inheriting this interface
}

type Book implements LibraryItem {
    title: String
    author: String
}
Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294
harshil-goel pushed a commit that referenced this pull request Jul 6, 2023
…oss all the implementing types. (#7710)

Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field.

Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field.

Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts.

If the interface argument is not present or its value is false then that field will be unique only for one implementing type.
But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602

Example Schema,

 interface LibraryItem {
    refID: String! @id                   //  This field is unique only for one implementing type
    itemID: String! @id(interface:true)    // This field will be unique over all the implementing types inheriting this interface
}

type Book implements LibraryItem {
    title: String
    author: String
}
Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294
harshil-goel pushed a commit that referenced this pull request Jul 6, 2023
…oss all the implementing types. (#7710)

Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field.

Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field.

Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts.

If the interface argument is not present or its value is false then that field will be unique only for one implementing type.
But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602

Example Schema,

 interface LibraryItem {
    refID: String! @id                   //  This field is unique only for one implementing type
    itemID: String! @id(interface:true)    // This field will be unique over all the implementing types inheriting this interface
}

type Book implements LibraryItem {
    title: String
    author: String
}
Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294
harshil-goel pushed a commit that referenced this pull request Jul 6, 2023
…oss all the implementing types. (#7710)

Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field.

Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field.

Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts.

If the interface argument is not present or its value is false then that field will be unique only for one implementing type.
But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602

Example Schema,

 interface LibraryItem {
    refID: String! @id                   //  This field is unique only for one implementing type
    itemID: String! @id(interface:true)    // This field will be unique over all the implementing types inheriting this interface
}

type Book implements LibraryItem {
    title: String
    author: String
}
Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294
harshil-goel added a commit that referenced this pull request Jul 17, 2023
…oss all implementing types (#8876)

Currently, @id fields in the interface are unique only in one
implementing type. But there are several use cases that require @id
field to be unique across all the implementation types. Also currently.
get a query on the interface can result in unexpected errors as we can
have multiple implementing types have the same value for that @id field.

Now we are allowing the @id field in the interface to be unique across
all the implementing types. In order to do this, we have added a new
argument interface of boolean type in the @id field.

Whenever a @id field in interface type has an interface argument set
then its value will be unique across all the implementing types. Users
will get errors if they try to add a node with such a field and there is
already a node with the same value of that field even in some other
implementing types. This is true for other scenarios like adding nested
value or while using upserts.

If the interface argument is not present or its value is false then that
field will be unique only for one implementing type. But such fields
won't be allowed in argument to get query on interface in the future,
see this PR also #7602

Example Schema,

 interface LibraryItem {
refID: String! @id // This field is unique only for one implementing
type
itemID: String! @id(interface:true) // This field will be unique over
all the implementing types inheriting this interface
}

type Book implements LibraryItem {
    title: String
    author: String
}
Related discuss Post:
https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294

Cherry picked from: #7710
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/graphql Issues related to GraphQL support on Dgraph.
Development

Successfully merging this pull request may close these issues.

2 participants